Fix equals nullability annotations for jspecify compliance#18930
Fix equals nullability annotations for jspecify compliance#18930therepanic wants to merge 1 commit intospring-projects:mainfrom
Conversation
In this commit, we added `@Nullable` to equals methods of classes that support `jspecify` for consistency with other Spring projects and to avoid bugs that caused other Spring projects to do this natively. Closes: spring-projectsgh-18929 Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
| if (obj == null) { | ||
| return false; |
There was a problem hiding this comment.
In some places this is necessary, otherwise the project will not build.
There was a problem hiding this comment.
I believe this one could also be addressed by fixing the unsafe type check that follows it. I'd recommend it just after the super.equals(obj) clause.
jzheaux
left a comment
There was a problem hiding this comment.
Thanks, @therepanic, for the PR and the underlying research that went into it.
Does this also close #18927? If so, you may consider adding that Closes to your commit as well. Otherwise, will you please elaborate on that ticket what work is needed in addition to this PR?
| @Override | ||
| public boolean equals(Object other) { | ||
| public boolean equals(@Nullable Object other) { | ||
| if (other == null) { |
There was a problem hiding this comment.
This one feels odd since I would imagine it best caught by a type check.
Can we remove this and also the unsafe type check by doing:
if (!(other instanceof DefaultCacheKey otherKey)) {
return false;
}| if (obj == null) { | ||
| return false; |
There was a problem hiding this comment.
I believe this one could also be addressed by fixing the unsafe type check that follows it. I'd recommend it just after the super.equals(obj) clause.
In this commit, we added
@Nullableto equals methods of classes that supportjspecifyfor consistency with other Spring projects and to avoid bugs that caused other Spring projects to do this natively.Closes: gh-18929